fix(daemon): verify structured process identity - #310

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

crates/daemon/src/supervisor.rs (1)

822-843: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Avoid the script-file read when the direct match already succeeds.

script_command_matches eagerly calls fs::read_to_string(command) on every ownership check — including the common direct-executable case where the result is discarded. verify_ownership runs on reconciliation paths, so this is avoidable disk I/O proportional to script size.

♻️ Short-circuit the script branch

 fn process_identity_matches(
     process_identity: &platform::ProcessIdentity,
     command: &Utf8Path,
     arguments: &[String],
 ) -> bool {
     let direct_arguments_match = process_identity.arguments == arguments;
     let direct_command_matches = process_identity.executable == command
         || (command == Utf8Path::new("/bin/sh")
             && process_identity.executable == Utf8Path::new("/bin/bash")
             && process_identity.argument_zero == command.as_str());
+    if direct_command_matches && direct_arguments_match {
+        return true;
+    }
+
     let script_command_matches = process_identity
         .arguments
         .first()
         .is_some_and(|script| script == command.as_str())
         && fs::read_to_string(command).is_ok_and(|source| source.starts_with("#!"));
     let script_arguments_match = process_identity
         .arguments
         .get(1..)
         .is_some_and(|live_arguments| live_arguments == arguments);
 
-    (direct_command_matches && direct_arguments_match)
-        || (script_command_matches && script_arguments_match)
+    script_command_matches && script_arguments_match
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/daemon/src/supervisor.rs` around lines 822 - 843, Update
process_identity_matches so the script-specific checks are evaluated only after
the direct command-and-arguments match fails, preserving the existing direct
match result while avoiding fs::read_to_string(command) for successful direct
matches. Keep the current script matching behavior unchanged for non-direct
matches.

crates/platform/src/process/unsupported.rs (1)

4-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant Err(...?) wrapping.

Since unsupported(...) already returns Result<T, PlatformError> for any T, this can be returned directly without the Err(...?) indirection (which makes the outer Err(...) unreachable in practice once unsupported returns its error).

♻️ Proposed simplification (verify against sibling unsupported shims for consistency first)

 pub(super) fn inspect_process_identity(
     _pid: u32,
 ) -> Result<Option<ProcessIdentity>, PlatformError> {
-    Err(unsupported(PlatformCapability::ProcessInspection)?)
+    unsupported(PlatformCapability::ProcessInspection)
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/platform/src/process/unsupported.rs` around lines 4 - 8, Update
inspect_process_identity to return
unsupported(PlatformCapability::ProcessInspection) directly, removing the
redundant Err wrapper and question-mark propagation while preserving the
existing Result<Option<ProcessIdentity>, PlatformError> signature and
unsupported behavior.

crates/platform/src/process.rs (1)

39-41: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use a top-level import instead of the fully qualified crate::PlatformError.

macos.rs and unsupported.rs both use crate::PlatformError; at the top and reference it unqualified; this function inlines the fully qualified path instead.

As per coding guidelines, "PREFER top-level imports over local imports or fully qualified names in Rust."

♻️ Proposed fix

+use crate::PlatformError;
+
 pub fn inspect_process_identity(pid: u32) -> Result<Option<ProcessIdentity>, crate::PlatformError> {
+pub fn inspect_process_identity(pid: u32) -> Result<Option<ProcessIdentity>, PlatformError> {
     implementation::inspect_process_identity(pid)
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/platform/src/process.rs` around lines 39 - 41, Update
inspect_process_identity to import PlatformError at the module level and use the
unqualified PlatformError in its return type, matching the style used by
macos.rs and unsupported.rs.

Source: Coding guidelines